[web] Render PlatformViews with SLOT tags. #25747

ditman · 2021-04-24T00:45:16Z

Render PlatformViews in SLOT tags. To do so:

Wraps most of the DOM from a flutter app in the Shadow DOM of flt-glass-pane (was [web] Wrap all the contents of the glass-pane in a shadow root. #25483)
Unifies the Platform View lifecycle in a PlatformViewManager class. That class keeps track of platform view factory functions, their rendered output, and the SLOTs for the Shadow DOM.
- Previously a PlatformView was a single DOM node, but now it has been split in two:
  - The slot(which flutter is able to move around within the render tree).
  - The content (which is always a direct child of the flt-glass-pane element, and contains the rendered output of a platform view factory).
Platform messages related to platform views are now handled by a PlatformViewMessageHandler class, which is configured in the engine's platform_dispatcher.dart. Platform Messages for Platform Views are now more easily testable.
Unused code across the engine has been removed (everything related to the old way of handling platform views in canvaskit and html renderers).

Fixes

[web] Use ShadowDOM slots to render HtmlElementViews flutter#80524

Testing

Deployed the Gallery App using this version of the engine: https://dit-gallery.web.app
All existing tests pass.
Added new tests for the new classes added.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.
The reviewer has submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

harryterkelsen

Mostly LGTM with some nits and test fixes

lib/web_ui/lib/src/engine/dom_renderer.dart

lib/web_ui/lib/src/engine/platform_dispatcher.dart

lib/web_ui/lib/src/engine/platform_views/content_manager.dart

lib/web_ui/test/engine/platform_views/content_manager_test.dart

yjbanov

lib/web_ui/lib/src/engine/dom_renderer.dart

lib/web_ui/lib/src/engine/platform_views/content_manager.dart

lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart

harryterkelsen

mostly LGTM

harryterkelsen · 2021-05-17T21:07:45Z

lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart

-      _releaseOverlay(unusedViewId);
-      _rootViews[unusedViewId]?.remove();
-    }
+    disposeViews(unusedViews);


I'm not sure this new code will actually dispose of "disposed" views. In the old code we kept a list of views which were disposed, and then later we disposed them. In this code we are only disposing views which are trying to be composited into the scene but were never registered. In the old code we would delete disposed views whether or not they are being actively composited this frame.

Ah yes, the thing is that in the old code this was coupled with the creation/destruction of the actual content of the platform view in the DOM.

Now, slots are "lightweight" so they can be destroyed/recreated per frame without impacting performance, and the actual platform view (with the heavy iframe) will still get destroyed when the framework signals a disposal.

Can you think of some example where this code might be leaking unused views?

yjbanov

lib/web_ui/lib/src/engine/platform_views/content_manager.dart

lib/web_ui/lib/src/engine/platform_views/message_handler.dart

lib/web_ui/lib/src/engine/platform_views/slots.dart

…It messes with clip counts :)

…ws in a frame can be reported later by submitFrame.

… non-functioning platform view when under testing)

…plifies stuff. See what breaks!

…tead of Map<dynamic, dynamic>?.

Remove caching ability from Slots (only required for the canvaskit rendering, will be added there)

…nd the clipCount of views in the canvaskit backend.

…ion everywhere.

)

Replace the per-platform-view shadow dom query with a top-level shadow dom query. Fixes breakage from flutter/engine#25747

flutter-dashboard bot added the platform-web Code specifically for the web engine label Apr 24, 2021

google-cla bot added the cla: yes label Apr 24, 2021

This was referenced Apr 27, 2021

[web] Wrap all the contents of the glass-pane in a shadow root. #25483

Closed

[web] Use ShadowDOM slots to render HtmlElementViews flutter/flutter#80524

Closed

ditman changed the title ~~[WIP] [web] Render PlatformViews with SLOT tags.~~ [web] Render PlatformViews with SLOT tags. May 12, 2021

ditman requested review from yjbanov and harryterkelsen May 12, 2021 21:29

ditman marked this pull request as ready for review May 12, 2021 21:30

harryterkelsen approved these changes May 12, 2021

View reviewed changes

harryterkelsen approved these changes May 13, 2021

View reviewed changes

yjbanov approved these changes May 15, 2021

View reviewed changes

ditman commented May 17, 2021

View reviewed changes

lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart Show resolved Hide resolved

ditman requested review from yjbanov and harryterkelsen May 17, 2021 20:50

harryterkelsen approved these changes May 17, 2021

View reviewed changes

yjbanov approved these changes May 18, 2021

View reviewed changes

lib/web_ui/lib/src/engine/platform_views/content_manager.dart Outdated Show resolved Hide resolved

lib/web_ui/lib/src/engine/platform_views/message_handler.dart Outdated Show resolved Hide resolved

lib/web_ui/lib/src/engine/platform_views/slots.dart Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

ditman mentioned this pull request May 19, 2021

[web] Make web integration tests shadow DOM aware. flutter/flutter#82926

Merged

8 tasks

This comment has been minimized.

Sign in to view

ditman added 11 commits May 20, 2021 12:04

Wrap all the contents of the glass-pane in a shadow root.

6ea7e48

Make analyzer happy

61ed861

Tweak semantics_test so it is shadow-DOM aware.

5e1720d

Render all text_editing primitives inside the new shadow root.

7b76403

Tweak text_editing_test so it is shadow-DOM aware.

0d11719

dartfmt -w .

b5b4ec8

Do not select flt-glass-pane all the time. Reuse the reference inside .

149debf

Don't mess with the dart version in tests

a1de790

Revert fanciness

6daf5c6

Ensure the glass-pane and its shadow root exist before testing.

e51ad1d

Make dom_renderer_test shadow-root aware.

bb97136

ditman added 20 commits May 20, 2021 12:09

Make content height:100% if it isn't set by the user. Warn the user.

0d41b5c

Prevent re-creating the same view multiple times in the CK renderer. …

a57a5c3

…It messes with clip counts :)

Make flt-glass-pane tag name configurable.

63370d5

Noop _compositeWithParams for disposed views, so all the disposed vie…

bb68314

…ws in a frame can be reported later by submitFrame.

Adapt tests to look for PlatformViews in their new locations.

1203eb3

Volkswagen the framework tests (replicate old behavior of returning a…

15bc192

… non-functioning platform view when under testing)

Decouple rendering of platform view contents and slots lifecycle, sim…

8c5a13a

…plifies stuff. See what breaks!

Remove unused var.

3c55ce8

Add unit tests for the new platform_views classes.

38d3191

Appease analyzer

6397962

Make semantics/text_field_test Shadow-DOM aware.

314e246

Add licenses for new files.

518b2e2

Address PR feedback.

ea60e8c

Fix embedded_views.dart imports.

d060e90

Use Object? as the type for the ParameterizedPlatformViewFactory, ins…

4bca48c

…tead of Map<dynamic, dynamic>?.

Separate slot rendering from platform view content management.

3d819ca

Remove caching ability from Slots (only required for the canvaskit rendering, will be added there)

Add ViewClipChain class to encapsulate the clip root, the contents, a…

4959eea

…nd the clipCount of views in the canvaskit backend.

Fix semantics_test after rebase.

8d0bac1

Ensure user supplied content is correctly sized.

1697d17

Update Dartdoc so it is aware of the new createPlatformViewSlot funct…

f374b1f

…ion everywhere.

ditman added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label May 21, 2021

fluttergithubbot merged commit b5f8141 into flutter:master May 21, 2021

ditman deleted the platform-views-in-slots-no-flag branch May 21, 2021 00:33

engine-flutter-autoroll mentioned this pull request May 21, 2021

Roll Engine from acea7c424dc5 to e7f0ef028929 (3 revisions) flutter/flutter#83066

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 21, 2021

b5f8141 [web] Render PlatformViews with SLOT tags. (flutter/engine#25747

339ab5e

)

stuartmorgan mentioned this pull request May 21, 2021

[url_launcher] Fix Link test breakage from shadow DOM changes flutter/plugins#3961

Merged

11 tasks

stuartmorgan added a commit to flutter/plugins that referenced this pull request May 21, 2021

[url_launcher] Fix Link test breakage from shadow DOM changes

77cd1fc

Replace the per-platform-view shadow dom query with a top-level shadow dom query. Fixes breakage from flutter/engine#25747

ditman mentioned this pull request Jun 10, 2021

[web] App crashes on startup when browser doesn't support ShadowDOM flutter/flutter#84383

Closed

fotiDim pushed a commit to fotiDim/plugins that referenced this pull request Sep 13, 2021

[url_launcher] Fix Link test breakage from shadow DOM changes

f759644

Replace the per-platform-view shadow dom query with a top-level shadow dom query. Fixes breakage from flutter/engine#25747

henkibro pushed a commit to henkibro/url_launcher that referenced this pull request Mar 1, 2022

[url_launcher] Fix Link test breakage from shadow DOM changes

09161a5

Replace the per-platform-view shadow dom query with a top-level shadow dom query. Fixes breakage from flutter/engine#25747

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[web] Render PlatformViews with SLOT tags. #25747

[web] Render PlatformViews with SLOT tags. #25747

ditman commented Apr 24, 2021 •

edited

Loading

harryterkelsen left a comment

yjbanov left a comment

harryterkelsen left a comment

harryterkelsen May 17, 2021

ditman May 17, 2021

yjbanov left a comment

This comment has been minimized.

This comment has been minimized.

[web] Render PlatformViews with SLOT tags. #25747

[web] Render PlatformViews with SLOT tags. #25747

Conversation

ditman commented Apr 24, 2021 • edited Loading

Fixes

Testing

Pre-launch Checklist

harryterkelsen left a comment

Choose a reason for hiding this comment

yjbanov left a comment

Choose a reason for hiding this comment

harryterkelsen left a comment

Choose a reason for hiding this comment

harryterkelsen May 17, 2021

Choose a reason for hiding this comment

ditman May 17, 2021

Choose a reason for hiding this comment

yjbanov left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

ditman commented Apr 24, 2021 •

edited

Loading